Skip to content

[OUDS] Link component#2836

Merged
vprothais merged 23 commits intoouds/mainfrom
ouds/main-sc-navigation-links
Mar 17, 2025
Merged

[OUDS] Link component#2836
vprothais merged 23 commits intoouds/mainfrom
ouds/main-sc-navigation-links

Conversation

@nilloq
Copy link
Member

@nilloq nilloq commented Jan 16, 2025

Related issues

#2824

Description

Implement default html link style and a new link component:

  • define default style in _reboot.scss and update Content > Reboot documentation accordingly
  • implement new _links.scss for component and create a new Components > Links documentation page
  • remove unuseful _icon-link.scss and _chevron-link.scss files for which contents are merged in the new _links.scss file

Motivation & Context

Base component of the library

Types of change

  • New feature

Live previews

@nilloq nilloq force-pushed the ouds/main-sc-navigation-links branch from 692f3e8 to 034d6d0 Compare February 11, 2025 14:56
Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not the final review.

  • I think I messed up with the // OUDS mod comments, need to recheck all those.
  • It's probably missing the migration guides before merging.
  • I haven't checked all the occurrences of links in the doc.
  • Do we handle redirections to the last versions of /utilities/link and /helpers/icon-link ?
  • I haven't spend much time to investigate the different variants of the links when being standalone.

@nilloq nilloq force-pushed the ouds/main-sc-navigation-links branch from d256e57 to 0a043a0 Compare February 24, 2025 11:29
Base automatically changed from ouds/main-his-btn-comp to ouds/main February 26, 2025 11:20
@louismaximepiton louismaximepiton force-pushed the ouds/main-sc-navigation-links branch from 378e9cd to 48fc2cc Compare February 26, 2025 17:30
@netlify
Copy link

netlify bot commented Feb 26, 2025

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 32199ab
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/67d7d33399f2cb0008ac0aad
😎 Deploy Preview https://deploy-preview-2836--boosted.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@vprothais vprothais requested a review from Copilot February 27, 2025 08:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR introduces a new link component and updates related documentation to align with the redesigned default link styles, while cleaning up outdated files and examples. Key changes include:

  • The addition of a new components documentation page (links.md) with examples on base, chevron, and icon links.
  • Updates to the reboot documentation to reflect disabled and visited link styles.
  • Adjustments to the sidebar and typography docs by removing legacy or commented-out link information.

Reviewed Changes

File Description
site/content/docs/0.1/components/links.md New documentation with examples for the reusable link component styles and variants.
site/content/docs/0.1/content/reboot.md Updated examples to demonstrate disabled link styling and visited link handling.
site/data/sidebar.yml Updated sidebar to include new Links entries and removal of outdated link topics.
site/content/docs/0.1/content/typography.md Removed obsolete commented sections regarding links to streamline documentation.

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

site/content/docs/0.1/components/links.md:4

  • [nitpick] The word 'customer' appears to be a typo; if the intended meaning is to describe tailored or custom link styles, consider correcting it to 'custom'.
description: Use OUDS Web's customer link styles for links as a key navigational element, enabling users to move between pages, sections, or external resources.

Copy link
Member

@louismaximepiton louismaximepiton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a pseudo element to increase the tap size ?
I think we can reintroduce .btn-link with the PR as well.

@nilloq nilloq marked this pull request as ready for review March 4, 2025 14:07
Copy link
Member

@hannahiss hannahiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I have a lot of remarks about doc, I tried to make some propositions... I also saw a few "bugs". I'll be available to explain if needed.

I put here some notes I didn't know where to write:

  • Missing .btn-link class to add in buttons page, maybe just in bootstrap compat, and .btn .btn-link may look like .link
  • I feel like something is missing about disabled state: in buttons we explain everything about aria-disabled, and href, and tabindex, but here nothing. Maybe this part should be in links and referenced in buttons?

@vprothais vprothais requested a review from Copilot March 6, 2025 15:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR introduces the new Links component along with updated default link styles, and migrates related documentation to reflect these changes.

  • Adds new documentation for the Links component and its variants.
  • Updates migration guides to include new Sass variables and announce breaking changes.
  • Removes legacy documentation and sidebar entries for deprecated link helpers.

Reviewed Changes

File Description
site/content/docs/0.2/components/links.md New documentation for the Links component and its usage variants
site/content/docs/0.2/migration.md Migration guide updated with new Sass variables and breaking changes
site/content/docs/0.2/migration-from-boosted.md Updated migration information reflecting the new Links component
site/content/docs/0.2/content/reboot.md Reboot documentation updated to demonstrate new link styles and disabled state
site/content/docs/0.2/components/buttons.md Reference to the disabled link accessibility warning updated
site/data/sidebar.yml Sidebar updated to remove legacy link entries
site/content/docs/0.2/content/typography.md Removal of obsolete commented-out link documentation

Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

site/content/docs/0.2/migration.md:57

  • The Sass variable '$ouds-link-color-content-pressed:' has a trailing colon, which is inconsistent compared to the other variable definitions; please verify if the colon is intended or if a correct value assignment is missing.
      <li><code>$ouds-link-color-content-pressed:</code></li>

Copy link
Member

@hannahiss hannahiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the visited style should be applied only on visited state
  • remove on colored bg variants for standalone links (do we have to make sure it doesn't apply to them? I think so...)
  • add an exmaple of on colored bg variants in reboot
  • styles on hover : to discuss
  • some wording

Copy link
Member

@hannahiss hannahiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok LGTM :-)

@vprothais vprothais merged commit efe2817 into ouds/main Mar 17, 2025
16 of 18 checks passed
@vprothais vprothais deleted the ouds/main-sc-navigation-links branch March 17, 2025 08:47
@github-project-automation github-project-automation bot moved this from In Progress / Draft to Done in 🟣 [Orange-Boosted-Bootstrap] PRs Board Mar 17, 2025
@louismaximepiton louismaximepiton linked an issue Mar 25, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

[OUDS] Component Creation - Link

5 participants